-
Notifications
You must be signed in to change notification settings - Fork 0
Development: Refine user data export
#1742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@sachmii Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
|
@sachmii Test coverage has been automatically updated in the PR description. |
|
@sachmii Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
|
🤖 No OpenAPI or client changes needed. |
Abi107717
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for implementing this 😊Code LGTM, Just small comments, is there are a reason, that you are using NON_NULL instead of NON_EMPTY?
src/main/java/de/tum/cit/aet/core/dto/exportdata/IntervieweeExportDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/core/dto/exportdata/InterviewProcessExportDTO.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/core/dto/exportdata/InterviewSlotExportDTO.java
Outdated
Show resolved
Hide resolved
…t/1695-add-interview-data-to-user-data-export
|
@sachmii Test coverage has been automatically updated in the PR description. |
|
🤖 No OpenAPI or client changes needed. |
|
@sachmii Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested it on the testserver and the Interview Data is not included in the export data, thank you!
One thing I noticed is that the time in the export data differs from the time displayed for the Interview slots. I think they use different time zones, which can be confusing for the user. Maybe you could adjust it in the exported data or add a time zone there.
Another nitpick I noticed is that the times for the Interview data are in the format: "date T time Z". For the comments, the format "date T time" is used, without the Z at the end. This is not very important, just something I noticed for consistency 😄
| InterviewProcess process = new InterviewProcess(); | ||
| process.setJob(job); | ||
| process = interviewProcessRepository.save(process); | ||
|
|
||
| Interviewee interviewee = new Interviewee(); | ||
| interviewee.setInterviewProcess(process); | ||
| interviewee.setApplication(application); | ||
| interviewee.setLastInvited(Instant.parse("2025-01-01T10:00:00Z")); | ||
| interviewee = intervieweeRepository.save(interviewee); | ||
|
|
||
| InterviewSlot slot = new InterviewSlot(); | ||
| slot.setInterviewProcess(process); | ||
| slot.setInterviewee(interviewee); | ||
| slot.setStartDateTime(Instant.now().plusSeconds(3600)); | ||
| slot.setEndDateTime(Instant.now().plusSeconds(7200)); | ||
| slot.setLocation("Room 101"); | ||
| slot.setStreamLink("https://stream.test/slot"); | ||
| slot.setIsBooked(true); | ||
| interviewSlotRepository.save(slot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create test data for the interview data?
Maybe you could also open a new issue for the other interview service tests, since they don’t seem to use test data yet 😅 (Or adjust them as well in this PR, but this is not necessary)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworked the tests in my other PR #1755 and I don't create an interview data there.
Regarding the format, I will implement this in the #1755 PR. Update: @Kiara65, I just implemented it in my latest commit in #1755. |
|
@sachmii Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
|
🤖 No OpenAPI or client changes needed. |
Kiara65
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick update, lgtm! 😄


Checklist
General
Server
Motivation and Context
Closes #1695, needed to add interview data to user data export/
Description
Steps for Testing
Prerequisites:
Review Progress
Code Review
Manual Tests
Test Coverage
Warning: Server tests failed. Coverage could not be fully measured. Please check the workflow logs.
Last updated: 2026-01-23 11:21:42 UTC